Skip to content

Birmingham | 26-ITP-May | Gabriel Pawuoi | Sprint 3 | Implement and rewrite tests - #1577

Open
KhotKeys wants to merge 6 commits into
CodeYourFuture:mainfrom
KhotKeys:sprint-3-implement-rewrite
Open

Birmingham | 26-ITP-May | Gabriel Pawuoi | Sprint 3 | Implement and rewrite tests#1577
KhotKeys wants to merge 6 commits into
CodeYourFuture:mainfrom
KhotKeys:sprint-3-implement-rewrite

Conversation

@KhotKeys

Copy link
Copy Markdown

Learners, PR Template

Self checklist

  • I have titled my PR with Region | Cohort | FirstName LastName | Sprint | Assignment Title
  • My changes meet the requirements of the task
  • I have tested my changes
  • My changes follow the style guide

Changelist

  • Completed coursework/implement-and-rewrite following the ReadMe instruction.

@KhotKeys KhotKeys added 📅 Sprint 3 Assigned during Sprint 3 of this module Needs Review Trainee to add when requesting review. PRs without this label will not be reviewed. Module-Structuring-And-Testing-Data The name of the module. labels Jul 26, 2026

@cjyuan cjyuan left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code looks good.

Just two comments.

// TODO: Implement this function
const suit = card.slice(-1);
const rank = card.slice(0, -1);
const validSuits = ["\u2660", "\u2665", "\u2666", "\u2663"];

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why not just specify the suit characters as "♠", "♥", "♦", "♣"?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Alright, let me resolve this!

Comment on lines +12 to +25
test(`should return false when numerator >= denominator`, () => {
expect(isProperFraction(2, 1)).toEqual(false);
expect(isProperFraction(3, 3)).toEqual(false);
});

test(`should handle negative numerator`, () => {
expect(isProperFraction(-1, 2)).toEqual(true);
expect(isProperFraction(-3, 2)).toEqual(false);
});

test(`should handle negative denominator`, () => {
expect(isProperFraction(1, -2)).toEqual(true);
expect(isProperFraction(3, -2)).toEqual(false);
});

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could consider using pseudo-code and notations like abs(...) or | ... | in the descriptions to make them more informative and concise.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Alright,

@cjyuan cjyuan added Reviewed Volunteer to add when completing a review with trainee action still to take. and removed Needs Review Trainee to add when requesting review. PRs without this label will not be reviewed. labels Jul 27, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Module-Structuring-And-Testing-Data The name of the module. Reviewed Volunteer to add when completing a review with trainee action still to take. 📅 Sprint 3 Assigned during Sprint 3 of this module

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants